Skip to content

reuse visitField for conversions on intermediate types#52

Merged
nathanhhughes merged 2 commits intomainfrom
feature/better_conversions
Jul 18, 2025
Merged

reuse visitField for conversions on intermediate types#52
nathanhhughes merged 2 commits intomainfrom
feature/better_conversions

Conversation

@nathanhhughes
Copy link
Copy Markdown
Collaborator

Fixes to how conversions are handled to deal with the issue I was showing you. Mostly straightforward though I had to change getFieldInputInfo slightly. I think my solution works but idk if you can see a cleaner way to handle it (there's some implicit state from how visitField gets called that is hard to reason about)

@nathanhhughes nathanhhughes requested a review from Schmluk July 9, 2025 17:39
Copy link
Copy Markdown
Collaborator

@Schmluk Schmluk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I didn't check all input info in the greatest detail but looks correct from a first pass. Not sure input info for conversions is covered in the other tests but again looks correct from a first pass.


// Non-config types with a conversion.
template <typename Conversion, typename T, typename std::enable_if<!isConfig<T>(), bool>::type = true>
// Types with a extra conversion.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Always good to have less constraints. 🙂

@nathanhhughes nathanhhughes force-pushed the feature/better_conversions branch 2 times, most recently from 10c86ba to 0cb4b7f Compare July 17, 2025 15:31
Base automatically changed from feature/dynamic_configs_ros2 to main July 17, 2025 19:25
@nathanhhughes nathanhhughes force-pushed the feature/better_conversions branch from 0cb4b7f to a33024e Compare July 18, 2025 00:01
@nathanhhughes nathanhhughes merged commit 7986ffe into main Jul 18, 2025
1 check passed
@nathanhhughes nathanhhughes deleted the feature/better_conversions branch July 18, 2025 00:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants